The hourly timerfd reopen is emitted as a log-crate info! record. With the hypervisor's default verbosity (-v 0 → LevelFilter::Warn, see main.rs), info! is filtered out by log::set_max_level before it ever reaches common.rs::Logger, so this control record is a no-op and the timer never triggers a reopen. It also does nothing in --log-stderr (sync) mode, where common.rs drops LOG_CTRL_REOPEN records entirely (line 209) and writes bypass the decorator.

So the "control thread emits the LOG_CTRL_REOPEN control record once per hour" guarantee only holds when the log level is Info or higher AND async file logging is in use. That's fine in practice because the write-triggered identity check (reopen_if_needed_at/file_replaced) covers rename-based rotation within ~1s of the next write, but the docs currently present the timer as an unconditional hourly reopen. Worth either documenting the Info+ / async precondition, or emitting at a level that isn't filtered (or using a path that doesn't depend on the log facade's max level).

The PR description says "keep the current descriptor usable when reopen fails; retry on later writes with a one-second cooldown" — that behavior is implemented on the VMM side, but on the CubeShim side a failed reopen() propagates through ? and aborts the whole write_log_rotate invocation. The old descriptor is dropped when the function returns, and the consumer loop restarts the writer after a 3-second sleep with freshly-opened files.

Two consequences worth noting:

• log_writer.reopen() and stat_writer.reopen() are not independent: if the log-file reopen succeeds but the stat-file reopen fails (or vice-versa), the error tears down both writers even though one descriptor was already swapped, causing a 3s gap plus a redundant reopen of the healthy file.
• During the 3s restart window (or if the file remains unopenable and the 3s retry loop spins), the mpsc channel (cap 1024) can fill and try_send in Log::log/Log::stat silently drops messages.

This is recoverable, but it differs from the documented "keep old descriptor, retry with 1s cooldown" behavior. Consider matching the VMM's tolerant handling here (e.g. keep the old descriptor on reopen failure instead of ?), or adjusting the docs to describe the shim's restart behavior.

The PR description claims rename-based rotation is detected "on a subsequent write" via a device/inode comparison ("at most once per second", with "retry on later writes with a one-second cooldown"). This write path performs no such comparison — the old hourly wall-clock check was removed and nothing replaced it. Reopen now happens only when the hourly LOG_CTRL_REOPEN control record reaches this decorator (CubeShim has its own 30-min Rotate event). The docs (service-management.md) describe the schedule-driven behavior correctly, so only the PR body overstates the fast-path detection — but worst-case reopen latency is now bounded by the timer, not by writes.

The hourly reopen depends entirely on this control record surviving the slog_async channel (chan_size(8192), OverflowStrategy::Drop). Under heavy logging the record is silently dropped and the file stays rotated until the next hourly tick. There is also a second loss window in hypervisor/src/common.rs:158-160: records buffered before the logger thread starts are drained with a plain slog_info! (no #LOG_CTRL_REOPEN tag), so a reopen emitted in that window is written as a normal log line and never reopens the file. If a reopen is missed/delayed past the next delaycompress cycle, the writer can be appending to an inode logrotate is about to gzip (writes go to a deleted file) — silent data loss. Worth confirming a missed reopen is acceptable.

create_log_reopen_timer failure (e.g. EMFILE/ENFILE — the VMM opens many fds) now aborts VMM startup via Error::LogReopenTimerCreate before any VM is created. A logging-rotation convenience turning into a fatal boot failure is a regression; consider making the timer best-effort (log a warning and continue without periodic reopen) so fd exhaustion degrades rotation, not the VM.

? propagates a failed reopen() out of write_log_rotate, so the outer retry loop sleeps 3s during which the receiver is not drained; try_send (channel cap 1024) then drops log/stat messages. That drop window pre-exists for write errors, but reopen failures are a new, more likely trigger (e.g. logrotate create 0640 root root producing a file the shim's account can't reopen, which the docs explicitly warn about). Consider logging the reopen failure and continuing the loop instead of aborting it, so one failed reopen doesn't pause the writer.

This test does not exercise the reopen path and its assertions will fail. slog::record!(level, msg, args, b) puts the 2nd argument in the record message and leaves the tag as "" — it does not set a tag. PlainDecorator::with_record only takes the reopen branch when record.tag() == LOG_CTRL_REOPEN, so this control record (tag "") is treated as an ordinary record. Meanwhile RawFormat::format_compact writes only record.msg(): the "before"/"after" payloads above are passed as args (never written), and this record writes the literal string LogReopen to the still-open old fd. The rotated file ends up as "LogReopen" and the active file stays empty, so both assert_eq!s at lines 257–258 fail — i.e. cargo test -p logging --lib will fail, contradicting the PR's validation claim.

To exercise the path, the control record must carry the tag and the payloads must be in the msg position, e.g. slog::Record::with_tag(LOG_CTRL_REOPEN, Level::Info, "", &format_args!(""), b!()) (and record!(Level::Info, "before", &format_args!(""), b!()) for the ordinary records).

The whole reopen mechanism now depends on this warn! record surviving two filters that can silently drop it:

1. log facade max level. log::set_max_level(VmmConfig.log_level) is used at init (hypervisor/src/lib.rs:138). The CLI can't select Error, but VmmConfig.log_level = LevelFilter::Error via the library API would drop this record before it reaches PlainDecorator, so the VMM keeps appending to a renamed file that rotate 24 will eventually compress/delete out from under the still-open fd — unbounded hidden disk usage, with no log noise to indicate rotation broke.

2. Deferred-logger buffer. With the default config, defer_logger_thread == true (log_level <= Info, which includes the CLI default Warn), so common::Logger::log_async pushes records emitted before vcpu_started into self.buffer and replays them later via slog_info!(logger, "{}", v) without the #LOG_CTRL_REOPEN tag. If the hourly firing lands before vCPU start, that hour's reopen is skipped and the raw warning text is written to the file as an ordinary line; if vCPUs never start, control records are buffered indefinitely and the file is never reopened.

The warn! level is a deliberate hack to survive the facade filter; it would be more robust to route the reopen out-of-band (the timerfd could feed a channel/flag the control thread checks) or at least guard against log_level < Warn when creating the timer.

open_log_file and open_replacement_log_file are byte-for-byte identical (same create(true).append(true).open). The "replacement" semantics are just the same create+append open, so consolidate into a single helper and call it from both PlainDecorator::new and RawPlainDecorator::reopen.

open_replacement_log_file is byte-for-byte identical to open_log_file (both create(true) + append(true) + open). RawPlainDecorator::reopen can just call open_log_file and this second function can be removed — the "replacement" semantics the name suggests don't actually exist (the file is opened by path, not by swapping an existing handle).

This reopen control only works through a fragile chain: the log-crate record (warn!(target: ...)) is bridged to the slog tag by Logger::log_async in hypervisor/src/common.rs (slog_info!(..., #LOG_CTRL_REOPEN, ...)), and only then does PlainDecorator::with_record see record.tag() == LOG_CTRL_REOPEN.

Two consequences worth documenting or hardening:

1. In sync mode (log_stderr = true), Logger::log drops the control record entirely (common.rs:209), so the hourly timer fires but the decorator never reopens. Today sync mode writes to stderr rather than the file, so this is latent — but any future change that writes to a file in sync mode will silently lose rotation.
2. If the control record is ever emitted directly through slog without the #LOG_CTRL_REOPEN tag (e.g., the buffered-drain path at common.rs:159 drains without the tag), the reopen is silently skipped.

Consider having the VMM control thread call a dedicated reopen API on the decorator/logger instead of routing the signal through the log-facade record.

When reopen() fails here, ? exits write_log_rotate, the outer loop in consumer drops the ReopenableFile (closing the old descriptor), sleeps 3 s, and restarts. That contradicts the PR description's "keep the current descriptor usable when reopen fails; retry on later writes with a one-second cooldown": the shim's descriptor is closed on failure and the retry interval is 3 s (during which messages queue in the bounded 1024-slot channel and are silently dropped under load).

Since ReopenableFile::reopen already keeps self.file on failure, the Rotate arm could log the error and continue the loop, retrying at the next 30-minute Rotate — matching the stated design. If the current propagate-and-restart behavior is intended, the PR description should be updated.